Skip to content

Polish propchange event detail names#116

Merged
DmitrySharabin merged 3 commits into
rollback-signalsfrom
props-api-polish
May 15, 2026
Merged

Polish propchange event detail names#116
DmitrySharabin merged 3 commits into
rollback-signalsfrom
props-api-polish

Conversation

@DmitrySharabin
Copy link
Copy Markdown
Member

@DmitrySharabin DmitrySharabin commented May 15, 2026 •
edited
Loading

Rename propchange event detail fields to match main:

  • parsedValue → value (now the parsed/converted value; the raw input is no longer in the detail)
  • oldInternalValue → oldValue
  • attributeName is only set when an attribute is actually involved

Internal: Prop#set param oldValue → oldAttributeValue to avoid shadowing the new local; onprops.js and one propchange.js test follow the new names.

Stacked on #115. Baseline preserved: 91 pass / 7 fail / 4 skip (same 7 pins).

🤖 Generated with Claude Code

@DmitrySharabin DmitrySharabin mentioned this pull request May 15, 2026
Open
3 tasks
@DmitrySharabin
Populate detail on first_connected shortcut re-fires (#106)
fa139e7 
The re-fire previously dispatched a PropChangeEvent with no detail, so
a listener that read e.detail.value would crash on the catch-up
dispatch while succeeding on the regular dispatch. Populate a minimal
detail with source: "initial" (marker for synthetic catch-up) and
value (the field whose absence caused the crash).

Found while integration-testing nude-element against color-elements
(<space-picker value="oklab" onspacechange="…"> exercises this path).
@DmitrySharabin
Preserve explicit null in reflected attribute writes
b509235 
`change.attributeValue ?? change.element.getAttribute(...)` treated an
explicit null (callers signaling "clear this attribute") the same as
"no override given, look it up on the element." When a reflected prop
was set to null or undefined, the fallback re-read the existing
attribute, so the clear never happened.

Switch to `!== undefined` so `null` propagates to the subsequent
`attributeValue === null` branch and removes the attribute.

Fixes the "Clearing a reflected prop removes the attribute" regression
tests for both `value: undefined` and `value: null`.
Base automatically changed from props-bug-fixes to rollback-signals May 15, 2026 21:56
@DmitrySharabin @claude
Polish propchange event detail names
e82d409 
Rename the `propchange` event detail fields to match the post-signals
naming on `gitbutler/workspace` so consumers see a single, clearly named
value pair:

- `event.detail.parsedValue` → `event.detail.value` (now the parsed /
  converted value; the raw user-provided input is no longer in the
  detail)
- `event.detail.oldInternalValue` → `event.detail.oldValue`
- `event.detail.attributeName` is only set when an attribute is
  actually involved (was previously always present, sometimes
  undefined)

Internal supporting changes:

- `Prop#set` destructured param `oldValue` → `oldAttributeValue` so the
  new local `oldValue` (previous parsed value) doesn't shadow it
- `Props#attributeChanged` caller updated to pass
  `oldAttributeValue: oldValue`
- `src/plugins/events/onprops.js` consumer reads `change.value` /
  `change.oldValue`
- One `test/plugins/props/propchange.js` consumer updated

Baseline preserved: 91 pass / 7 fail / 4 skip — the 7 pre-existing
regression pins are unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@DmitrySharabin DmitrySharabin merged commit 2fc7690 into rollback-signals May 15, 2026
@DmitrySharabin DmitrySharabin deleted the props-api-polish branch May 15, 2026 22:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@LeaVerou LeaVerou LeaVerou approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@DmitrySharabin @LeaVerou